Build failure is due to codebase rot

#164 (comment)

Yeah, we're really lacking maintenance here, unfortunately. As always, I'm happy to submit PRs to tackle issues and implement features every once in a while in between other tasks, but need a second maintainer for review and approval.

@rib can you update us on your availability and commitment towards this crate, or should we seek for a third maintainer to fill this review-gap?

Regarding the PR itself, note that the result of a looper wake is very loosely defined and not to be relied on: #170

If there's any explicit async task spawning and progress monitoring that needs to happen on an ALooper, I'd prefer if we add an abstraction based on file descriptors or callbacks which, unlike waking, can be explicitly handled.

Interesting. Does it have a chance of spuriously not waking up the event loop? winit currently assumes it always does.

@notgull according to their documentation, it would get batched up with other events, such that the event loop is "active" / "awake" but without ever seeing it return WAKE because it got "folded into" the other events.

winit currently relies on Poll::Wake to trigger a user-requested redraw:

https://github.com/rust-windowing/winit/blob/ae4c449670674d8ac0d6d8754caf3fe5f4954c25/src/platform_impl/android/mod.rs#L586-L602

But it also checked these before entering a poll_events(): https://github.com/rust-windowing/winit/blob/ae4c449670674d8ac0d6d8754caf3fe5f4954c25/src/platform_impl/android/mod.rs#L562-L565

With these findings, it'll miss events on redraw_flag and proxy_wake_up, unless we hoist those checks out of the match block for any Poll/Err returned out of poll_events(). Or if we want to be more specific, attach a pipe or eventfd to be guaranteed a consistent result whenever our channel into the Looper signaled anything.

I would prefer to just use eventfd.

Same preference here, it's much more explicit than pretending every Poll could have a Wake folded into it.

For redraws specifically, there's a preference to handle these via AChoreographer intead, which would bump the API requirements. But that's for a future discussion.

Sorry for only just coming around to look at this.

This seems reasonable.

Re: #170 and the way that wake events are documented: the point is just that when _wake() is used to wake up a Looper then although it does guarantee a wake up (you don't have to worry about it not waking) it's not guaranteed that you specifically get a ::Wake event if there are other events that can be delivered after the wake up.

Regarding how winit handles Wake events, winit is doing the right thing already I believe.

Winit never specifically requires a ::Wake event for anything - it's kind of the opposite and it will ignore ::Wake events when it currently has nothing it needs to do.

In itself a Wake event is the the last-resort event that's delivered after the even loop wakes after calling epoll_wait if there is nothing else to report (such as a timeout or an idle callback or some other file descriptor event).

So from Winit's point of view a Wake event on its own doesn't represent anything interesting that needs to be handled, unless it knows that it requested a wake up.

The android backend for Winit effectively tracks when it's waiting for a wake up and so this code in Winit:

                android_activity::PollEvent::Wake => {
                    // In the X11 backend it's noted that too many false-positive wake ups
                    // would cause the event loop to run continuously. They handle this by
                    // re-checking for pending events (assuming they cover all
                    // valid reasons for a wake up).
                    //
                    // For now, user_events and redraw_requests are the only reasons to expect
                    // a wake up here so we can ignore the wake up if there are no events/requests.
                    // We also ignore wake ups while suspended.
                    self.pending_redraw |= self.redraw_flag.get_and_reset();
                    if !self.running
                        || (!self.pending_redraw && !self.user_events_receiver.has_incoming())
                    {
                        return;
                    }
                }

is about ignoring Wake events when Winit has nothing it needs to do.

In other words, an ALooper_wake() may lead to any event (it's only promising that epoll_wait or _pollOnce will unblock and return with an event in a finite amount of time. This is why the Android NDK docs clarify that you should treat any event like a Wake event. I suppose in the past they made the mistake of implying in their docs that _wake() will always result in a Wake which is not the case.

So in Winit specifically, it only has that special case for ignoring some Wake events and then apart from that, all events (including Wake events that are not ignored) will result in the same single_iteration code running.

I think trying to make a Waker based on ALooper is probably the right choice on Android.

This is the closest that the NDK officially lets you get to epoll and an ALooper is also the basis of their Java-side android.os.Looper and therefore also the mechanism by which it's also possible to drive wake ups for the Java main/UI thread.

So even if you tried to punch lower and use epoll directly on Android that would then not be able to drive the callbacks that can be registered with their widely-used Looper abstraction.

I would prefer to just use eventfd.

Btw, I believe you can use eventfd() on Android - bionic libc exposes it and the syscall is white listed.

A file descriptor from eventfd would be compatible with ALooper (re: ALooper_addFd) which internally is a wrapper around epoll_wait

ref: https://android.googlesource.com/platform/bionic/%2B/android-8.0.0_r4/libc/SECCOMP_WHITELIST.TXT#70

Looking at this I realized there's a soundness issue with the current implementation of AndroidAppWaker that may affect the implementation of this PR: #226

In particular we need to make sure AndroidAppWaker calls ALooper_acquire and it needs to implement Clone and Drop so it calls ALooper_acquire/release.

The same would apply to RawWakerVTable clone + drop functions which would need to make sure to call ALooper_acquire and ALooper_release or maybe put an AndroidAppWaker into an Arc so the looper can be ref-counted.